Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Transactions with readTime will omit begin and commit transaction requests, and instead pass readTime on individual read requests. #1565

Merged
merged 10 commits into from
Feb 14, 2024
Merged

fix: Transactions with readTime will omit begin and commit transaction requests, and instead pass readTime on individual read requests. #1565

merged 10 commits into from
Feb 14, 2024

Conversation

tom-andersen
Copy link
Contributor

@tom-andersen tom-andersen commented Feb 8, 2024 •
edited
Loading

Transactions with readTime do not need to begin and commit transaction. Instead, the individual requests can use readTime instead of transactionId. This will reduce latency when doing point in time reads.

@product-auto-label product-auto-label bot added size: l Pull request size is large. api: firestore Issues related to the googleapis/java-firestore API. labels Feb 8, 2024
@tom-andersen tom-andersen changed the title Tomandersen/read only transaction fix: Tomandersen/read only transaction Feb 8, 2024
@tom-andersen tom-andersen changed the title fix: Tomandersen/read only transaction fix: Transactions with readTime will omit begin and commit transaction requests, and instead pass readTime on individual read requests. Feb 10, 2024
@tom-andersen tom-andersen marked this pull request as ready for review February 10, 2024 19:40
@tom-andersen tom-andersen requested review from a team as code owners February 10, 2024 19:40
@tom-andersen tom-andersen added the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 10, 2024
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 10, 2024
@tom-andersen tom-andersen added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Feb 10, 2024
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 10, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 10, 2024
tom-andersen
tom-andersen commented Feb 12, 2024
View reviewed changes
Copy link
Contributor Author

@tom-andersen tom-andersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback. I think I have addressed all points, and pushed up changes.

...le-cloud-firestore/src/main/java/com/google/cloud/firestore/ServerSideTransactionRunner.java Show resolved Hide resolved
google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java Show resolved Hide resolved
...le-cloud-firestore/src/main/java/com/google/cloud/firestore/ServerSideTransactionRunner.java Show resolved Hide resolved
google-cloud-firestore/src/main/java/com/google/cloud/firestore/ServerSideTransaction.java Show resolved Hide resolved
milaGGL
milaGGL approved these changes Feb 12, 2024
View reviewed changes
Copy link
Contributor

@milaGGL milaGGL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with one quick question. readTime is added for Query and AggregateQuery, what about single documents?

@tom-andersen
Copy link
Contributor Author

one quick question. readTime is added for Query and AggregateQuery, what about single documents?

The proto doesn't support transaction or readTime on single get. Whenever transaction or readTime is required, we use batch get proto, even if it is for a single document.

@tom-andersen tom-andersen added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 12, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 12, 2024
@tom-andersen tom-andersen merged commit d5fb01a into main Feb 14, 2024
24 checks passed
@tom-andersen tom-andersen deleted the tomandersen/readOnlyTransaction branch February 14, 2024 14:33
@release-please release-please bot mentioned this pull request Feb 14, 2024
Merged
@release-please release-please bot mentioned this pull request May 17, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@milaGGL milaGGL milaGGL approved these changes

Assignees

@milaGGL milaGGL

Labels
api: firestore Issues related to the googleapis/java-firestore API. size: l Pull request size is large.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tom-andersen @milaGGL @yoshi-kokoro